Rewrite confirmation modals #1985
Conversation
niklasmohrin
force-pushed
the
rewrite-modals
branch
2 times, most recently
from
dd3cedd
to
943c7ce
Compare
niklasmohrin
force-pushed
the
rewrite-modals
branch
from
1068fc5
to
a79e7b3
Compare
niklasmohrin
force-pushed
the
rewrite-modals
branch
from
ae36e95
to
6edafce
Compare
niklasmohrin
force-pushed
the
rewrite-modals
branch
3 times, most recently
from
b94211d
to
a219e47
Compare
There was a problem hiding this comment.
Looking good! I like the declarative way to defining modals. The whole ElementInternals API is new to me, I can't really tell if the code is canonical, but it seems reasonable to me. It would still be nice to have frontend tests for that -- maybe one for a modal that can be directly submitted and one for semester deletion with the input validation?
|
(when converting new modals, it might make sense to do a separate commit for each modal so that reviewing becomes easier -- moved code is always hard to review) |
niklasmohrin
force-pushed
the
rewrite-modals
branch
from
407d4a4
to
3253038
Compare
|
I decided to leave the couple of contact modals as they are for now, because this PR is already huge |
evap/rewards/templates/rewards_reward_point_redemption_events.html
Outdated
Show resolved
Hide resolved
niklasmohrin
force-pushed
the
rewrite-modals
branch
2 times, most recently
from
0dcf19f
to
04a8dda
Compare
evap/rewards/templates/rewards_reward_point_redemption_event_list.html
Outdated
Show resolved
Hide resolved
More realistic alternative to `current` Add custom elements scaffold looks right, doesn't open found a way to use import, but not have icons modal texts displayed before script is loaded fix usage of custom elements prototype before it was loaded make it open the modal dont know some progess with native dialogs make it look somewhat like bootstrap progress? make animations work implement confirmation logic ough, now the slots inherit values from the .card-header bar Use `:defined` instead of `.loaded` class more modals and fix blocktrans Allow putting extra stuff under `confirmation-modal` more modals format more modals
Co-authored-by: Johannes Wolf <[email protected]>
…on an `<a>` ancestor, even if the dialog stops event propagation in a handler
niklasmohrin
force-pushed
the
rewrite-modals
branch
from
1cdb0a0
to
a6bdd98
Compare
|
Should I try to make a handful of useful commits out of this (not today), or should we just squash it? |
|
I'm fine with just squashing |
|
Should we create a follow-up issue for the contact modal and the delegate preparation modal? |
|
Sounds good! |
Closes #1851
Our modals have been bugging me for a while now and I believe the number of times I announced that I would rewrite them is just out of finger range. I am not sure how exactly it will look in the end, but I want it to be less Django magic and more browser native. My first vision here uses a custom web component at the use-site. This allows the logic of the modal to be together with the opening button. Furthermore, everything about the modal is contained in one HTML tag.
In my current plan, there won't be multiple users of one modal anymore, but instead multiple copies of the same template (specified by the custom web component). This will keep HTML size similar to what we had before, but increase DOM size significantly on sites with many modals (for example lists with deletable entries). We will have to see the effect this has on performance, I would believe that modern browsers should be able to handle this, but you never know. If it turns out to be slow, I would divert to trying to implement some kind of
reference-modalcomponent for sharing DOM nodes.Thoughts?